Skip to content

refactor(benchmark): decide the experiment identity in one owner - #5359

Open
karenchuu wants to merge 4 commits into
loopx-project:mainfrom
karenchuu:codex/benchmark-experiment-identity
Open

refactor(benchmark): decide the experiment identity in one owner#5359
karenchuu wants to merge 4 commits into
loopx-project:mainfrom
karenchuu:codex/benchmark-experiment-identity

Conversation

@karenchuu

@karenchuu karenchuu commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Goal And Delivered Outcome

  • Outcome basis / anchor: no pre-existing issue or board row. The anchor is measured duplication, two parts of which no name-keyed scan can see. On the intended base a9ee074de:

    • the token shape ^[A-Za-z0-9][A-Za-z0-9_.:@+-]{0,127}$ was compiled five times in five modules -- four under the name _TOKEN_RE (study_projection.py:44, experiment_board.py:21, concurrency_envelope.py:20, factorial_contrast.py:17) and once as traex_evidence.py:18 _PUBLIC_TOKEN: same value, different name.
    • above four of those five, a byte-identical private helper was restated: def _token(value: Any, *, field: str) -> str with the same body and the same rejection text "{field} must be a compact public-safe token" -- 90 call sites across the five modules.
    • the arm-role vocabulary {baseline, control, treatment, explore} was defined four times in four modules: three _ARM_ROLES set literals (study_projection.py:45, experiment_board.py:22, concurrency_envelope.py:21) and one plain list in the CLI that admits a benchmark slot, loopx/cli_commands/benchmark_concurrency.py:105 choices=["baseline", "control", "treatment", "explore"].
    • Consequence: the vocabulary the CLI accepts and the envelope that admits the run were separate restatements of one decision, and widening the token in one toolkit module left the other four rejecting the same value.

  • Observable before → after: loopx/capabilities/benchmark_toolkit/experiment_identity.py holds ARM_ROLES, ARM_ROLE_CHOICES, EXPERIMENT_TOKEN_PATTERN and experiment_token_text, and six modules ask it. All nine local definitions are gone (the diff removes exactly those lines), for a product-side diff of +54 / -54 across seven files -- net zero lines, counting the 33-line owner module. No accepted value and no rejection message changes: the guard asserts each consumer's _token is the owner's function object and each arm check uses the owner's frozenset.

  • Intended base: a9ee074de.

Scope And Continuation

  • Done: the owner, six migrated modules, the CLI vocabulary, and a 49-case architecture guard.
  • ARM_ROLE_CHOICES is stated, not derived. Those four names reach argparse as choices, and their order is the order a human sees in --help; tuple(sorted(ARM_ROLES)) would silently reorder it. The guard pins the literal order and checks the set of the tuple against ARM_ROLES.
  • Not collapsed, and asserted so in the guard rather than left to be "finished" wrongly:
    • anchor_role not in {"baseline", "control", "treatment"} (experiment_board.py:264) and ARM_ROLES - {"baseline"} are subsets and derivations -- a second real decision about anchor arms, not a duplicate of the vocabulary. The guard carries both as probe cases that must not be reported, plus a probe that promotes a subset to the full set and must then be reported.
    • integrity.py:137 _PUBLIC_EVIDENCE_ID_PATTERN and extensions/presentation.py:46 _ID_RE share a bound but differ in allowed characters (@, +). They are the two sites refactor(public-safety): centralize compact identifier shapes #5351 lists under "Not in scope" and belong with that owner.
    • four_arm_contract.py:112-124 writes "arm_role": arm_role into the arm dict without asking the owner whether the role is one of the four. Making it validate would newly reject specs that build today, so that is a decision for the four-arm contract owner; this PR only makes the missing check visible.
  • Policy note, from the repository's own gate: loopx canary premerge classifies this diff as benchmark_sensitive and holds self-merge ("benchmark adapter, scoring, runner, or evidence paths require explicit maintainer review before self-merge"). All nine selected checks executed with zero failures; the hold is reported here rather than worked around.
  • Slice boundary / successor: complete within scope. Reverting is mechanical -- the owner has no other callers and each migrated module lost one constant and one helper.

Validation

  • Tested revision: 170489354 (4 commits, 8 files, +631 -54; product side seven files +54 / -54, guard 577 lines added).
  • Run state: finished.
  • Input classes: synthetic fixtures; no live benchmark run and no provider calls.
Check kind Result Public-safe evidence / limitation
unit passed pytest tests/architecture/test_benchmark_experiment_identity_owner.py -> 49 passed in 4.7s: value scan over loopx/, owner values pinned against literals, per-consumer object identity plus a real reference in the module body, the CLI's choices read back from its own syntax tree, 7 accept / 10 reject token cases including the 128-character bound, and 15 spelling probes (10 that must be reported, 5 that must not).
integration passed pytest tests/architecture tests/canary in full at this head: 1120 passed, 0 failed in 2m08s at 170489354 (the guard change touched only the new test file, and the count is the same as at d9ca66c44).
regression_parity passed pytest over the 23 test files that import a migrated module -> 626 passed, 9 skipped, 0 failed in 55s (the 9 skips are the toolkit's live-run guards). Head has no failure to attribute, so no base replay was needed. No test file was edited.
static passed python -m ruff check on all eight changed paths: clean. python -m mypy (no arguments, as CI runs it): Success: no issues found in 19 source files. git diff --check: clean.
static passed loopx check --scan-path for each of the eight changed paths through this tree's own entrypoint: ok: true, "public boundary scan clean: 8 files"; both warnings concern the absent local .loopx/registry.json.
semantics budget passed examples/semantic-vocabulary-drift-smoke.py on an unmodified a9ee074de worktree and on this head under identical conditions (same venv, Node 22.23.2 first on PATH, the same node_modules): output byte-identical -- conflicting_definitions=55/55, conflicting_values=16/16, multi_value_twins=8/8, same_runtime_forks=11/11, twins_raw=45. Disclosure of why removing nine duplicates moved no ratchet, since that is unusual for this track: neither vocabulary is in the inventory's counted kinds -- python_closed_sets contains 0 entries named _ARM_ROLES, ARM_ROLES, _TOKEN_RE or ARM_ROLE_CHOICES on base -- so conflicting_definitions was blind to them before and is after. No budget anchor was edited, and no constant name introduced here collides with an existing one.
canary passed (with the policy hold above) loopx canary premerge with the changed files passed explicitly: selected 9 / executed 9 / failures 0 / warnings 0, status: manual_review_required, manual_hold_count: 1, hold kind benchmark_sensitive.
mutation passed 13 mutations, one at a time in a separate worktree at this head, every file restored before each round, control round green (49 passed) before and after: 12 caught / 1 survived. Caught: a converted module restating the regex and the helper (M1, 2 cases); the same value under a different name with the owner import left in place (M2) -- re.compile caches by pattern and flags, so the identity assertion passes on a restated copy and only the value scan sees it; a consumer rebinding the arm vocabulary locally (M3); the CLI reverting to its own choices list (M4, 2 cases); a new module assembling the shape from same-file constants (M5); the vocabulary as frozenset(...) inside a function (M6); the owner widening the token bound (M7, 3 cases incl. the envelope suite); the owner reordering the CLI vocabulary (M8); the owner rewording the rejection text (M9, 13 cases); an anchor subset silently becoming the full vocabulary (M11); a declared site padded to the wrong count (M12). Closed by self-check: M10 -- a consumer re-adding def _token above the alias import -- first survived, because the import rebinds the name so the copy is dead code and my binding check only read ast.Assign targets. The last commit extends that check to module-level def and class names, and M10 now fails (1 case). Survived, and why it is equivalent: M13 replaces role not in ARM_ROLES with role not in set(ARM_ROLES) -- same membership answer for every input, wasteful but not a second owner; recorded instead of hidden.
frontend none No UI surface, projection or rendered output changes; loopx benchmark concurrency-admit --help text is unchanged because the choices order is pinned.

Type Of Change

  • Internal refactor / single-owner convergence with an anti-regression guard. No behaviour change, no dependency change.

LoopX Area

  • loopx/capabilities/benchmark_toolkit/*, loopx/cli_commands/benchmark_concurrency.py, loopx/capabilities/benchmark_toolkit/experiment_identity.py (new), tests/architecture/.

Technical Direction

  • Enumerate by folded value, not by name: _PUBLIC_TOKEN and the CLI's choices list are both invisible to a scan that keys on the constant name, and both were part of the same decision.
  • A shape and its rejection path travel together. Owning only the regex would have left five copies of the same validator -- the same half-measure the digest owner had to correct for.
  • argparse order is disclosed rather than derived: a "single source" that changes help text is a product change wearing a refactor's clothes.

Boundary Checklist

  • Neither the diff nor this PR body/comments/attachments disclose private state, credentials, raw traces or verifier output, internal links, or local machine paths.
  • I did not duplicate maintainer-owned benchmark work unless a maintainer split out a public issue for it.
  • I kept the change scoped to the linked issue/task.
  • I completed the visual evidence section for UI changes, or marked UI impact none.

@karenchuu
refactor(benchmark): state the experiment identity in one owner
428d572 
An arm role and a public-safe experiment token are validated at the toolkit
boundary, at the study projection, at the concurrency envelope and at the CLI
that admits a case slot. The token shape was compiled five times, once under a
different name, and four of those modules also carried a byte-identical private
_token helper with the same rejection text.

The ordered CLI vocabulary is stated rather than derived, because the order is
part of the help text a human reads.

Signed-off-by: karenchuu <25980598+karenchuu@users.noreply.github.com>
@karenchuu
refactor(benchmark): point the toolkit and CLI at the identity owner
8d56b21 
Five modules drop their local regex and _token helper and call the owner's, and
the three arm-role sets plus the CLI's argparse choices list read the same
frozenset and tuple. Nine local definitions go away for a net 33 fewer lines on
the product side; no accepted value and no rejection message changes.

Subsets and derivations of the arm vocabulary stay at their call sites: an
anchor may not be an explore arm, which is a second decision, not a duplicate.

Signed-off-by: karenchuu <25980598+karenchuu@users.noreply.github.com>
@karenchuu
test(architecture): guard the benchmark experiment identity owner
d9ca66c 
Both vocabularies are judged on folded values: any literal set, tuple, list or
frozenset call of the four arm names is an offender wherever it appears, and so
is a regex whose folded value is the token shape, at module level, inside a
function, or assembled from same-file string constants.

Consumers are wired by object identity plus a real reference in the module body,
and the CLI's choices are read back from its own syntax tree so a reordered or
re-listed vocabulary fails.

Signed-off-by: karenchuu <25980598+karenchuu@users.noreply.github.com>
@karenchuu
test(architecture): reject a shadowing local helper copy in a consumer
1704893 
Signed-off-by: karenchuu <25980598+karenchuu@users.noreply.github.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@huangruiteng huangruiteng Awaiting requested review from huangruiteng huangruiteng is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@karenchuu